The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props glendaviesnz, ramonopoly, jonsurrell, dmsnell.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

FYI - I will add tests for this once there is confirmation that this is the correct approach for fixing this bug. There may be a better solution. If there is feel free to close this PR and open an alternative.

Related: https://core.trac.wordpress.org/changeset/61486 / #10641 fixed the same class of KSES-mangling issue for Global Styles custom CSS by pre-escaping the JSON with JSON_HEX_TAG | JSON_HEX_AMP. This PR addresses the same problem for per-block custom CSS (attrs.style.css), which goes through the separate filter_block_kses() pipeline. I don't think the same pre-escaping approach can work in this case, as parse_blocks() calls json_decode() on the entire attributes object, which converts \u0026 back to & before KSES runs - but I do not know a lot about these flows, and don't have time to look closer as currenlty travelling - so could be completely wrong about this.

I had some trouble reproducing the issue.

In order to test this, I had to enable a recent version of the Gutenberg plugin. The individual block CSS feature is not yet available in Core yet, is it?

When I used an author role, I don't see the additional CSS panel for blocks. When I used an editor role, I was unable to reproduce the issue because it seems to have unfiltered_html capability already.

Am I doing something wrong in the reproduction steps?

Some thoughts based on the issue:

• KSES is unsuitable for processing data that is not HTML. This type of issue appears again and again.
• The KSES filters were added in r46896 / 7c38cf1. That seems to be a security fix with limited public information.
• I wish we could avoid applying KSES (HTML) filtering everywhere, but that seems unlikely at this time.
• I see "double-encoding" mentioned a few times. That doesn't seem accurate given the description. CSS text that will be used in rawtext STYLE (where HTML character references like & are not used) has had HTML character reference escaping applied to it. & appears in the CSS text because the character references will never be decoded in this context. It seems more accurate to talk about "mis-encoded" or even just "mangled." This is akin to applying any other unsuitable escaping mechanism.

I was very happy with the solution in r61486. Post content exclusively contained JSON which has some flexibility in escaping. JSON can be made to be plain HTML text by escaping HTML syntax characters (<>&). KSES ignores this. This approach escapes data before KSES can mangle it.

This PR tries to recover after KSES has mangled the data. That seems inherently more risky. It should be possible to decode HTML character references (like done here) but what if KSES starts to remove things that look like tags? What if I'd like to use content: '<data> here';? (KSES will likely strip <data> from this).

Another option is to protect the data before KSES can mangle it by encoding it ourselves in an HTML-text safe way. A few quick options come to mind:

• base64 the CSS string
• JSON encode the string with serializeAttributes()

Of course, before using the value it will need to be decoded appropriately. Either of these seem likely to prevent the issue by ensuring HTML syntax <>& are not present, so KSES should not take any action.

I had some trouble reproducing the issue.
In order to test this, I had to enable a recent version of the Gutenberg plugin. The individual block CSS feature is not yet available in Core yet, is it?
When I used an author role, I don't see the additional CSS panel for blocks. When I used an editor role, I was unable to reproduce the issue because it seems to have unfiltered_html capability already.
Am I doing something wrong in the reproduction steps?

@sirreal when logged in as the author user you do not need to see the custom CSS input box, you just need to edit the post content with the existing custom CSS in place that was added when you created the post as the admin user. This bug only occurs if a user without unfiltered_html edits a post that had block level customCSS add by a higher level user.

A few quick options come to mind:

Thanks a lot @sirreal, this is great.

Could "don’t run KSES on block attribute attrs.style.css" be another option?

Above, I was thinking of an allowlist of “non-HTML” attribute paths that are not HTML, e.g. ['css'], and in filter_block_kses_value(), when the current path is in that list, use some other sanitizer, e.g. wp_strip_all_tags or a variant of it

If there are hidden gotchas there...

protect the data before KSES can mangle it by encoding it ourselves in an HTML-text safe way

Maybe @glendaviesnz can answer this: let's say we encode in filter_block_kses (and decode when output in custom-css.php), do we need to worry about backwards compat at all? For example, for folks that have already used this feature in the plugin or elsewhere, would we have to infer “is this encoded or plain?”

when logged in as the author user you… need to edit the post content with the existing custom CSS in place

Got it, that worked. I did have to change the post author so that the author role user could edit the post.

Could "don’t run KSES on block attribute attrs.style.css" be another option?

That's a way to prevent this issue. The problem is that exceptions like that often create vulnerabilities. If a bad actor knows attrs.style.css will not be sanitized, they can often find a way to abuse it.

would we have to infer “is this encoded or plain?”

this is going to be a dead-end, because it’s largely not possible to do that. we can build in signals into the storage to communicate it though. for instance, prefix a base64-encoded string.

or in that same vein but better, store the attribute as a data URI which says explicitly what the content is.

{
	"style": {
		"css": "data:text/css;base64,eyBjb2xvcjogcmVkOyB9"
	}	
}

for the sake of transparency we can always escape the CSS from any characters that would be “dangerous,” but I think we’ve seen a number of cases where this has gone wrong because downstream code likes to unescape and re-escape, which ends up eliminating the escaping we intentionally applied.

wp_strip_all_tags

wp_strip_all_tags() is never going to be appropriate for CSS, but CSS should still go through some process like KSES, which is what functions like safecss_filter_attr() are for. there are rules applied to things like URLs inside of CSS declarations which WordPress will want to apply.

the $context parameter of filter_block_kses_value() offers a potential place to raise the bar on CSS handling. if we had a sentinel value indicating that the attribute is supposed to be CSS we could apply more appropriate sanitization, but we would want to make sure we don’t make it easy for people to set that context from user-supplied inputs.

Why are these attributes allowed at all for folks without the appropriate capability?

• The panel is not displayed for those users, suggesting that the intention is to prevent them from adding the CSS.
• This PR and ticket 64771 indicate that they do have access to author the CSS.

That is incoherent. If they can use the feature, let's show the UI (and make sure it works correctly). Otherwise, they should not be able to add custom CSS at all.

I've just confirmed that an author can add this and access the feature.

<!-- wp:paragraph {"style":{"css":"color: blue"}} -->
<p class="has-custom-css">asdf</p>
<!-- /wp:paragraph -->

How about a completely different approach:

• Strip an individual block's custom CSS for users without the correct capability.
• Display a warning in the editor on blocks that have custom CSS for users that will cause the custom CSS to be lost. "Warning: This block contains custom CSS and you do not have the appropriate capability. If you update this post, the custom CSS will be removed." (or something along those lines).

This PR and ticket 64771 indicate that they do have access to author the CSS.
That is incoherent.

Apologies, the wording of the ticket and PR was unclear, I have updated it to
Existing block level custom CSS in a post breaks when the post is edited by user without unfiltered_html to make it clearer.

Before finalising a fix for this, we probably need a decision on whether users without unfiltered_html should or should not be able to add/edit block-level custom CSS, eg. just show them the box if they can edit/add and fix the KSES issue, or just strip them and add the warning as @sirreal suggests.

I personally don't see any issue with allowing this user level to edit/add these attributes - seems very different to unfiltered_html to me - but this is not my area of expertise.

@ramonjd - any thoughts on the best way to get a decision on that?

Before finalising a fix for this, we probably need a decision on whether users without unfiltered_html should or should not be able to add/edit block-level custom CSS

My bag of 2c coins:

Here's the scenario I've been working with (I'm using authors as catch-all role for no unfiltered_html permissions):

1. An author creates a post called Y. Nice.
2. An admin/editor (or anyone with unfiltered_html permissions) logs in and edits post Y, adding custom CSS to a block.
3. Our author returns and edits anything in post Y (not custom CSS), then saves the post.
4. Custom CSS is mangled silently.

So they can "edit" it technically, because they can edit the post content, but in the regular editor UI flow they cannot. I get @sirreal's point about incoherency.

Following that I see the choice between:

A) keep the status quo and deal with CSS integrity preservation/kses mangling when saving the post OR
B) opening up custom CSS to users without unfiltered_html permissions OR
C) stripping with a warning (a general rule for life!)

With A, we have the very helpful suggestions from John Jon, Dennis and folks.

In relation to B, I'm not sure we'd want to add new, potential security holes. Authors can't currently add CSS, so we probably shouldn't let them. Happy to be persuaded on this and all points.

As for C, my instinct was that stripping wouldn't be appropriate, because authors by default can't see the custom css field (at least in my testing), so from their point of view they're not editing that attribute at all. And editors might wonder why the custom CSS they created is broken or stripped, BUT I was chatting with @tellthemachines, who made a good point to check the HTML block's behaviour in this regard.

Authors can't add CSS/JSS, and <style> tags are stripped. Any subsequent changes made by editors to the same block will be flagged in the editor the next time an author attempts to save the post:

So stripping would be more consistent with that block, and also JohnJon's "alternative" approach. The only difference is that it's not immediately visible to the author (without some sort of warning).

any thoughts on the best way to get a decision on that?

Extending authors' permissions, I expect, would be something that needs to be run past the core team and security folks.

Honestly, I think the quickest and safest approach right now is the HTML-block/Sirreal™️ approach because:

• it's consistent with existing flows (HTML block)
• it maintains current security/permission arrangements

Optionally, there could be some help text underneath the UI control to tell editors that

I say "for now" because maybe there's a better way down the road that preserves permissions and the intentions of admins when they are making changes, and, furthermore, promotes CSS content as generally safe under the right conditions.

I threw up a rough PR to help my brain work through this:

• Try/strip custom block css without unfiltered html gutenberg#76167

Unfortunately, stripping it is not a good solution for us in our multi-site scenario where the site admins do not have unfiltered_html permissions, and the block custom CSS was going to be generated by AI agents. We might just have to abandon the plans we had for this if stripping it is the only option for now.

We might just have to abandon the plans we had for this if stripping it is the only option for now.

I don't have a strong opinion. Maybe block css is the exception and it can be preserved?

Also, sorry @sirreal I spelled your name wrong (Jon with an h).